-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LNbits 0.12.x fixes #854
base: master
Are you sure you want to change the base?
LNbits 0.12.x fixes #854
Conversation
- change docker image name lnbits-legend to lnbits to match vendor naming
8fb040d
to
3676e59
Compare
use .env.example from LNbits package as .env template
add LNbits to settings page, fetch superuser usr and reset database
3156c6d
to
d9cc901
Compare
fi | ||
|
||
# if current LNbits version config is missign create .env from .env.example | ||
if [ ! -f /mnt/hdd/mynode/lnbits/update_config_$LNBITS_VERSION ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically, I would create a version number to track config updates so that it doesn't update automatically each upgrade / lnbits version change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the .env template be updated on each update so new options and defaults would be corrected on update too?
There are version files from Application Management related to installed, latest and available version. Could those be used as reference here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It normally doesn't need to be updated for each new release. Normally, the software should account for "missing" settings in the config file to use the default anyway. Keeping the file around makes it easy to update the default settings and control how the app runs. Those files just control the application version and not the settings files. There are examples in a few places, like pre_rtl.sh.
RTL_CONFIG_UPDATE_NUM=1
if [ ! -f /mnt/hdd/mynode/rtl/update_settings_$RTL_CONFIG_UPDATE_NUM ]; then
cp -f /usr/share/mynode/RTL-Config.json /mnt/hdd/mynode/rtl/RTL-Config.json
touch /mnt/hdd/mynode/rtl/update_settings_$RTL_CONFIG_UPDATE_NUM
fi
cp /usr/share/mynode/lnbits.env /mnt/hdd/mynode/lnbits/.env | ||
# Delete old myNode .env file created before 0.12.x | ||
if [ -f /usr/share/mynode/lnbits.env ]; then | ||
rm -f /usr/share/mynode/lnbits.env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not get rid of this file since I can control it a bit better and reference the config. Can it just be updated with the latest version from the lnbits repo? If I do have to put in default settings, this file (/usr/share/mynode/lnbits/env) is a nice way to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix the way you suggested
LNbits version bump 0.12.4 to be added later. |
#================================== | ||
# LNbits Functions | ||
#================================== | ||
def delete_lnbits_settings(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also don't think this would work. It looks like this launches a new container rather than running a command in the existing lnbits container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried it? It may work by launching a second container. Alternatively, you could exec into the running container to run the command, but I guess that would only clear the settings if lnbits was actively running.
info = "" | ||
|
||
info += to_string(subprocess.check_output("cat /mnt/hdd/mynode/lnbits/.super_user", shell=True)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this file that stored the super_user info get created? I only see it getting read/deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it looks like it file is created by lnbits.
I tested this and ported a subset of the changes in 75a2a38 The old config seems to still work well. |
Enhance MyNodeBTC to better support new LNbits features
Description
New LNbits versions after 0.12. have
A) new docker image name.
On vendor site new docker name
lnbits
instead oldlegend-lnbits
This PR changes docker image name to
lnbits
and there fore streamlines names.B) configuration file.
.env
has been included in mynodebtc setup. It doesn't reflect new features of lnbits.During start of lnbits service the new
.env
-configuration file is created from vendor provided file (.env.example
)C) web admin UI.
It's more simple than making configuration changes onto .env file on Linux console.
We set admin UI enabled now by default. After clean install user must setup username and password for superuser.
D) functions to reset settings database and reveal superuser access key (usr ID)
Repairing corrupted or messed settings and viewing superuser usrID web access keys need access to Linux console and some long command. like:
also file "
.super_user
" includes usrid when user has been created.This PR provides LNbits tablet onto Settings page where node operator has buttons for above functions
This PR solves mostly issue #848
NOTE!
Implementation is rude IMHO, but it works for me now. And it has lot more to improve later on. One additional change could be enable/disable admin UI with the switch as a workaround for LNbits open issue above.
Checklist
List of test device(s)
Raspi4b